Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

replacing sassc with dartsass-sprockets #92

Merged
merged 5 commits into from
Jul 14, 2024
Merged

replacing sassc with dartsass-sprockets #92

merged 5 commits into from
Jul 14, 2024

Conversation

jonmchan
Copy link
Contributor

@jonmchan jonmchan commented Jul 11, 2024 •
edited
Loading

sassc is deprecated. Replacing sassc with dart-sass is the simplest and most straightforward methodology to keep using sass. The gem dartsass-sprockets provides a drop-in replacement implementing almost everything that sassc-rails offered utilizing the same namespace and method signatures.

There are several deprecation errors that needs to be addressed with the newer version of sass, but it would be good to upgrade to the latest version of the library.

These errors can be addressed in a future PR.

Deprecation errors:

    /usr/local/bundle/bundler/gems/rails_email_preview-0babc0ff21e4/app/assets/stylesheets/rails_email_preview/_default.sass 60:17   @import
    /usr/local/bundle/bundler/gems/rails_email_preview-0babc0ff21e4/app/assets/stylesheets/rails_email_preview/application.scss 1:9  root stylesheet
Deprecation Warning: Using / for division outside of calc() is deprecated and will be removed in Dart Sass 2.0.0.

Recommendation: math.div($rep-grid-gutter, -2) or calc($rep-grid-gutter / -2)

More info and automated migrator: https://sass-lang.com/d/slash-div

   ╷
61 │   margin-right: ($rep-grid-gutter / -2)
   │                  ^^^^^^^^^^^^^^^^^^^^^
   ╵
    /usr/local/bundle/bundler/gems/rails_email_preview-0babc0ff21e4/app/assets/stylesheets/rails_email_preview/_default.sass 61:18   @import
    /usr/local/bundle/bundler/gems/rails_email_preview-0babc0ff21e4/app/assets/stylesheets/rails_email_preview/application.scss 1:9  root stylesheet
Deprecation Warning: Using / for division outside of calc() is deprecated and will be removed in Dart Sass 2.0.0.

Recommendation: math.div($rep-grid-gutter, 2) or calc($rep-grid-gutter / 2)

More info and automated migrator: https://sass-lang.com/d/slash-div

   ╷
67 │   padding-right: ($rep-grid-gutter / 2)
   │                   ^^^^^^^^^^^^^^^^^^^^
   ╵
    /usr/local/bundle/bundler/gems/rails_email_preview-0babc0ff21e4/app/assets/stylesheets/rails_email_preview/_default.sass 67:19   @import
    /usr/local/bundle/bundler/gems/rails_email_preview-0babc0ff21e4/app/assets/stylesheets/rails_email_preview/application.scss 1:9  root stylesheet
Deprecation Warning: Using / for division outside of calc() is deprecated and will be removed in Dart Sass 2.0.0.

Recommendation: math.div($rep-grid-gutter, 2) or calc($rep-grid-gutter / 2)

More info and automated migrator: https://sass-lang.com/d/slash-div

   ╷
68 │   padding-left: ($rep-grid-gutter / 2)
   │                  ^^^^^^^^^^^^^^^^^^^^
   ╵
    /usr/local/bundle/bundler/gems/rails_email_preview-0babc0ff21e4/app/assets/stylesheets/rails_email_preview/_default.sass 68:18   @import
    /usr/local/bundle/bundler/gems/rails_email_preview-0babc0ff21e4/app/assets/stylesheets/rails_email_preview/application.scss 1:9  root stylesheet
Deprecation Warning: Using / for division outside of calc() is deprecated and will be removed in Dart Sass 2.0.0.

Recommendation: math.div($col, 12) or calc($col / 12)

More info and automated migrator: https://sass-lang.com/d/slash-div

   ╷
75 │       width: percentage($col / 12)
   │                         ^^^^^^^^^
   ╵

@glebm
Copy link
Owner

glebm commented Jul 12, 2024

There appear to be quite a few Sass engines out there. Perhaps we can take a similar approach to the bootstrap ruby gem and support more of them?

https://github.com/twbs/bootstrap-rubygem/blob/main/lib/bootstrap/engine.rb

@jonmchan
Copy link
Contributor Author

@glebm I'd be happy to update the PR with the library loading script. I'll add similar verbiage to the README as the bootstrap gem.

@jonmchan
Copy link
Contributor Author

jonmchan commented Jul 13, 2024 •
edited
Loading

Ruby 3.0 and below are end of life and no longer supported. Should legacy versions of ruby be removed from the testing matrix? Only Rails 7.1, 7.0, and 6.1 is receiving support from the official rails team. Should Rails 5.0 and 4.2 be removed from the testing matrix?

@glebm
Copy link
Owner

glebm commented Jul 13, 2024

Yep, please remove them

glebm
glebm reviewed Jul 13, 2024
View reviewed changes
rails_email_preview.gemspec Outdated Show resolved Hide resolved
@jonmchan jonmchan force-pushed the dartsass-sprockets branch from b94492c to 07238af Compare July 13, 2024 14:38
@jonmchan
Copy link
Contributor Author

@glebm there's a dependency issue with rails_7_1 and ruby_version 3.1, otherwise all the tests pass. Here is the failed run:

https://github.com/wayhomeservices/rails_email_preview/actions/runs/9921493773/job/27409522524

I commented it out in the PR. I'm not 100% sure it is worth tracking down or not.

@jonmchan jonmchan force-pushed the dartsass-sprockets branch 4 times, most recently from d01bd0f to 9a4dd0b Compare July 13, 2024 23:49
@jonmchan jonmchan force-pushed the dartsass-sprockets branch from 9a4dd0b to 20bae12 Compare July 13, 2024 23:56
glebm
glebm reviewed Jul 14, 2024
View reviewed changes
lib/rails_email_preview.rb Outdated Show resolved Hide resolved
glebm
glebm reviewed Jul 14, 2024
View reviewed changes
README.md Show resolved Hide resolved
jonmchan and others added 2 commits July 14, 2024 14:28
@jonmchan @glebm
Update lib/rails_email_preview.rb
a9bd199 
Co-authored-by: Gleb Mazovetskiy <glex.spb@gmail.com>
@jonmchan
Update README.md
ce9c378
@glebm glebm merged commit a446fc6 into glebm:main Jul 14, 2024
4 of 5 checks passed
@glebm
Copy link
Owner

glebm commented Jul 14, 2024

Thank you for your effort and patience, Jonathan! Merged!

@jonmchan jonmchan deleted the dartsass-sprockets branch July 18, 2024 04:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@glebm glebm glebm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jonmchan @glebm